feat: validate-agents cobre squads e reporta YAML inválido em vez de engolir - #816
feat: validate-agents cobre squads e reporta YAML inválido em vez de engolir#816GiovaneLaurencio wants to merge 3 commits into
Conversation
…ents The squad's 8 agents carried the same GREENFIELD GUARD as the core agents, with the same two defects plus a consistency problem of their own. Agent Authority (Constitution Article II): 4 of the 8 recommended `*environment-bootstrap`, a @devops-exclusive command none of them owns (hooks-architect, mcp-integrator, skill-craftsman, swarm-orchestrator). They now delegate: `@devops *environment-bootstrap`. The 3 agents that recommend a command they actually own keep it unchanged — `*configure` (config-engineer), `*integrate-project` (project-integrator), `*check-updates` (roadmap-sentinel); each was verified to exist in its own `commands` block. False positive in non-project directories: the guard fired wherever there was no git repo, including a user home directory, and labelled it a "Greenfield project". It now decides from the working directory path already present in the system prompt (zero extra I/O): a home or non-project directory gets a tip to activate from inside the project instead of a command recommendation. claude-mastery-chief, which never recommended anything, gets the tip branch only. Typographic consistency: the three guard lines existed in 4 different variants across the 8 files (`--` vs `—`, with and without bold, with and without emoji). All 8 now carry byte-identical lines, matching the core agents. Note: `npm run validate:agents` covers only the 12 core agents, not squads. A manual YAML parse of all 8 found swarm-orchestrator.md invalid at its `lexicon:` block — pre-existing (fails identically at HEAD, before this change) and left untouched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 8 entries under `voice_dna.lexicon` had the shape:
- "topology" (preferred over "structure" for agent arrangements)
YAML closes the scalar at the second quote, leaving ` (preferred ...)` as a
stray token — js-yaml rejected the whole embedded block with "bad indentation
of a sequence entry", so the agent definition was unparseable by any tooling.
Each entry is now wrapped in single quotes, which keeps the inner double quotes
verbatim and keeps the parenthetical part of the value rather than turning it
into a comment:
- '"topology" (preferred over "structure" for agent arrangements)'
Verified: all 8 squad agents now parse, and lexicon still yields 8 entries with
their text unchanged. Entries elsewhere in the squad of the form
`- "text" # comment` were left alone — the `#` makes those valid already.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…engolir `npm run validate:agents` validava só os 12 agentes core. Squads não tinham gate nenhum — foi por isso que 4 variantes tipográficas do mesmo guard e um bloco YAML que nenhum parser aceita conviveram sem ninguém notar (ver SynkraAI#815). Pior: o loader tinha um try/catch em volta do loop INTEIRO. Um YAML quebrado abortava a leitura de todos os agentes seguintes, imprimia a falha como console.error e o run ainda terminava com "All validations passed". O gate existia no papel. O que muda: - descobre `squads/<squad>/agents/` além do core. Sumário por escopo: 20 agentes = core 12 + squad:claude-code-mastery 8. - NOVO check "YAML Parse": bloco que não parseia é ERRO (exit 1) com o motivo e uma sugestão. try/catch agora é POR ARQUIVO — um quebrado não esconde os outros. Verificado com um agente propositalmente inválido: acusou, saiu 1, e seguiu validando os 20. - unicidade de comando passa a ser POR ESCOPO. Core disputa com core, cada squad com ele mesmo — mesmo comando em dois squads diferentes não é colisão. Sem isso, ligar squads geraria falso-positivo em massa. - dependências de squad resolvem em `squads/<squad>/<type>/`; `reference_files` (que guardam caminho relativo à raiz) passam a ser checados contra o repo em vez de virar "unknown dependency type". - `--core-only` restringe ao comportamento antigo. - removida uma função morta que o refactor deixou referenciando variáveis inexistentes. `loadAllAgents()` mantém a assinatura antiga (só a lista) para não quebrar consumidor; `loadAgents()` é o novo, com parseErrors e escopos. Estado atual do repo: 0 erros, 121 warnings — todas dependências ausentes pré-existentes no core, nenhuma introduzida aqui. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@GiovaneLaurencio is attempting to deploy a commit to the SINKRA - AIOX Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe agent validator now discovers and validates core and squad agents with scoped command and dependency rules, YAML parse reporting, and a ChangesScoped agent validation
Greenfield activation guidance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant AgentLoader
participant Validators
participant ResultFormatter
CLI->>AgentLoader: Load core and optional squad agents
AgentLoader->>Validators: Provide agents and YAML parse errors
Validators->>Validators: Check scoped commands and dependencies
Validators->>ResultFormatter: Provide validation results and scope counts
ResultFormatter->>CLI: Display errors, warnings, and summary
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.aiox-core/infrastructure/scripts/validate-agents.js (1)
642-669: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
--helpoutput doesn't mention the new--core-onlyflag.The top-of-file JSDoc usage block (lines 21-23) and CLI parsing (line 664) both support
--core-only, but the interactive--help/-htext still only lists--jsonand--fix-suggestions. Users running--helpwon't discover the new flag.📝 Suggested fix
Usage: - node validate-agents.js Validate all agents + node validate-agents.js Validate core + squad agents + node validate-agents.js --core-only Validate only the 12 core agents node validate-agents.js --json Output as JSON node validate-agents.js --fix-suggestions Show fix suggestions🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.aiox-core/infrastructure/scripts/validate-agents.js around lines 642 - 669, Update the help text in main() to include the supported --core-only option alongside the existing CLI usage entries, matching the flag already parsed in the options object and documented by the top-level JSDoc..aiox-core/development/tasks/validate-agents.md (1)
72-81: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStep 5 wasn't updated for squad-local dependency resolution.
Step 1 and the Dependencies section now document that squad agents resolve
tasks/checklistsagainstsquads/<squad>/<type>/, but Step 5 still only mentions the core.aiox-core/development/...directories, leaving the dependency-validation spec internally inconsistent.📝 Suggested fix
For each agent's `dependencies.tasks` list: -1. Check that each referenced task file exists in `.aiox-core/development/tasks/` +1. Check that each referenced task file exists in `.aiox-core/development/tasks/` + (or `squads/<squad>/tasks/` for squad-scoped agents) 2. Report missing dependencies as ERRORS For each agent's `dependencies.checklists` list: -1. Check in `.aiox-core/development/checklists/` +1. Check in `.aiox-core/development/checklists/` + (or `squads/<squad>/checklists/` for squad-scoped agents) 2. Report missing as WARNINGSAs per path instructions, "Validate any referenced dependencies exist in the codebase" for
.aiox-core/development/tasks/**, and this dependency-resolution documentation should stay consistent with the rest of the task spec.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.aiox-core/development/tasks/validate-agents.md around lines 72 - 81, Update Step 5 in the dependency-validation instructions to resolve each agent’s task and checklist references using the squad-local directories documented elsewhere, while retaining `.aiox-core/development/tasks/` and `.aiox-core/development/checklists/` as applicable core locations. Keep missing tasks as ERRORS and missing checklists as WARNINGS, and make the validation paths consistent with the task’s existing dependency-resolution rules.Source: Path instructions
🧹 Nitpick comments (1)
.aiox-core/infrastructure/scripts/validate-agents.js (1)
677-684: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePortuguese-language comment amid an English codebase.
The comment above
loadAllAgentsis in Portuguese while the rest of the file's comments are English, which could hinder readability for non-Portuguese-speaking maintainers.♻️ Suggested fix
- // loadAllAgents mantém a assinatura antiga (devolve só a lista) para não quebrar - // quem já consome; loadAgents é o novo, com parseErrors e escopos. + // loadAllAgents keeps its original signature (returns only the list) to avoid + // breaking existing consumers; loadAgents is the new API, with parseErrors and scopes. loadAllAgents,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.aiox-core/infrastructure/scripts/validate-agents.js around lines 677 - 684, Translate the comment above the exported loadAllAgents and loadAgents symbols from Portuguese into clear, concise English, preserving its explanation that loadAllAgents retains the legacy list-only signature while loadAgents adds parseErrors and scopes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.aiox-core/infrastructure/scripts/validate-agents.js:
- Around line 121-168: The loadAgentsFromDir function should distinguish file
read failures from YAML parsing failures. Handle fs.readFile errors separately
with an appropriate read-error type and message, then keep
extractYamlFromMarkdown errors classified as INVALID_YAML with the existing YAML
suggestion.
- Around line 235-249: Update commandName to detect explicit-format objects
using Object.hasOwn(cmd, 'name'), including empty names, and return cmd.name ||
undefined. Prevent malformed explicit commands from reaching the shorthand
Object.keys branch and producing a bogus command name.
- Around line 325-343: Constrain `reference_files` resolution in the `depType
=== 'reference_files'` block so entries containing traversal cannot escape
`REPO_ROOT`. Resolve the path, validate that it remains within `REPO_ROOT`, and
only call `fileExists` or expose `expectedPath` for contained paths; handle
escaping entries as missing or invalid references using the existing warning
flow.
---
Outside diff comments:
In @.aiox-core/development/tasks/validate-agents.md:
- Around line 72-81: Update Step 5 in the dependency-validation instructions to
resolve each agent’s task and checklist references using the squad-local
directories documented elsewhere, while retaining
`.aiox-core/development/tasks/` and `.aiox-core/development/checklists/` as
applicable core locations. Keep missing tasks as ERRORS and missing checklists
as WARNINGS, and make the validation paths consistent with the task’s existing
dependency-resolution rules.
In @.aiox-core/infrastructure/scripts/validate-agents.js:
- Around line 642-669: Update the help text in main() to include the supported
--core-only option alongside the existing CLI usage entries, matching the flag
already parsed in the options object and documented by the top-level JSDoc.
---
Nitpick comments:
In @.aiox-core/infrastructure/scripts/validate-agents.js:
- Around line 677-684: Translate the comment above the exported loadAllAgents
and loadAgents symbols from Portuguese into clear, concise English, preserving
its explanation that loadAllAgents retains the legacy list-only signature while
loadAgents adds parseErrors and scopes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd51256f-2228-491d-9e21-6e70ff8abb2c
📒 Files selected for processing (11)
.aiox-core/development/tasks/validate-agents.md.aiox-core/infrastructure/scripts/validate-agents.js.aiox-core/install-manifest.yamlsquads/claude-code-mastery/agents/claude-mastery-chief.mdsquads/claude-code-mastery/agents/config-engineer.mdsquads/claude-code-mastery/agents/hooks-architect.mdsquads/claude-code-mastery/agents/mcp-integrator.mdsquads/claude-code-mastery/agents/project-integrator.mdsquads/claude-code-mastery/agents/roadmap-sentinel.mdsquads/claude-code-mastery/agents/skill-craftsman.mdsquads/claude-code-mastery/agents/swarm-orchestrator.md
| async function loadAgentsFromDir(dir, scope, depDirs) { | ||
| const agents = []; | ||
| const parseErrors = []; | ||
|
|
||
| let files; | ||
| try { | ||
| const files = await fs.readdir(AGENTS_DIR); | ||
| for (const file of files) { | ||
| if (file.endsWith('.md') && !file.startsWith('_')) { | ||
| const filePath = path.join(AGENTS_DIR, file); | ||
| const content = await fs.readFile(filePath, 'utf-8'); | ||
| const parsed = extractYamlFromMarkdown(content); | ||
|
|
||
| if (parsed?.agent) { | ||
| agents.push({ | ||
| file, | ||
| path: filePath, | ||
| id: parsed.agent.id || file.replace('.md', ''), | ||
| name: parsed.agent.name, | ||
| commands: parsed.commands || [], | ||
| dependencies: parsed.dependencies || {}, | ||
| parsed, | ||
| }); | ||
| } | ||
| files = await fs.readdir(dir); | ||
| } catch { | ||
| return { agents, parseErrors }; // directory absent — nothing to validate | ||
| } | ||
|
|
||
| for (const file of files) { | ||
| if (!file.endsWith('.md') || file.startsWith('_')) continue; | ||
|
|
||
| const filePath = path.join(dir, file); | ||
| try { | ||
| const content = await fs.readFile(filePath, 'utf-8'); | ||
| const parsed = extractYamlFromMarkdown(content); | ||
|
|
||
| if (parsed?.agent) { | ||
| agents.push({ | ||
| file, | ||
| path: filePath, | ||
| scope, | ||
| depDirs, | ||
| id: parsed.agent.id || file.replace('.md', ''), | ||
| name: parsed.agent.name, | ||
| commands: parsed.commands || [], | ||
| dependencies: parsed.dependencies || {}, | ||
| parsed, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| parseErrors.push({ | ||
| type: 'INVALID_YAML', | ||
| scope, | ||
| agent: file.replace('.md', ''), | ||
| file, | ||
| path: filePath, | ||
| message: `Unparseable YAML in ${scope}/${file}: ${String(error.message).split('\n')[0]}`, | ||
| suggestion: | ||
| 'Fix the embedded YAML block. A common cause is a sequence entry like `- "term" (note)`, where the text after the closing quote is a stray token — wrap the whole entry in single quotes.', | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| console.error(`Error reading agents directory: ${error.message}`); | ||
| } | ||
|
|
||
| return { agents, parseErrors }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Read failures are mislabeled as YAML parse errors.
The try/catch wraps both fs.readFile and extractYamlFromMarkdown. If the read itself fails (permissions, file removed mid-scan), the catch still reports INVALID_YAML with a YAML-fixing suggestion, which misleads whoever investigates the report.
🐛 Suggested fix
} catch (error) {
parseErrors.push({
type: 'INVALID_YAML',
scope,
agent: file.replace('.md', ''),
file,
path: filePath,
- message: `Unparseable YAML in ${scope}/${file}: ${String(error.message).split('\n')[0]}`,
+ message: `Failed to load ${scope}/${file}: ${String(error.message).split('\n')[0]}`,
suggestion:
'Fix the embedded YAML block. A common cause is a sequence entry like `- "term" (note)`, where the text after the closing quote is a stray token — wrap the whole entry in single quotes.',
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function loadAgentsFromDir(dir, scope, depDirs) { | |
| const agents = []; | |
| const parseErrors = []; | |
| let files; | |
| try { | |
| const files = await fs.readdir(AGENTS_DIR); | |
| for (const file of files) { | |
| if (file.endsWith('.md') && !file.startsWith('_')) { | |
| const filePath = path.join(AGENTS_DIR, file); | |
| const content = await fs.readFile(filePath, 'utf-8'); | |
| const parsed = extractYamlFromMarkdown(content); | |
| if (parsed?.agent) { | |
| agents.push({ | |
| file, | |
| path: filePath, | |
| id: parsed.agent.id || file.replace('.md', ''), | |
| name: parsed.agent.name, | |
| commands: parsed.commands || [], | |
| dependencies: parsed.dependencies || {}, | |
| parsed, | |
| }); | |
| } | |
| files = await fs.readdir(dir); | |
| } catch { | |
| return { agents, parseErrors }; // directory absent — nothing to validate | |
| } | |
| for (const file of files) { | |
| if (!file.endsWith('.md') || file.startsWith('_')) continue; | |
| const filePath = path.join(dir, file); | |
| try { | |
| const content = await fs.readFile(filePath, 'utf-8'); | |
| const parsed = extractYamlFromMarkdown(content); | |
| if (parsed?.agent) { | |
| agents.push({ | |
| file, | |
| path: filePath, | |
| scope, | |
| depDirs, | |
| id: parsed.agent.id || file.replace('.md', ''), | |
| name: parsed.agent.name, | |
| commands: parsed.commands || [], | |
| dependencies: parsed.dependencies || {}, | |
| parsed, | |
| }); | |
| } | |
| } catch (error) { | |
| parseErrors.push({ | |
| type: 'INVALID_YAML', | |
| scope, | |
| agent: file.replace('.md', ''), | |
| file, | |
| path: filePath, | |
| message: `Unparseable YAML in ${scope}/${file}: ${String(error.message).split('\n')[0]}`, | |
| suggestion: | |
| 'Fix the embedded YAML block. A common cause is a sequence entry like `- "term" (note)`, where the text after the closing quote is a stray token — wrap the whole entry in single quotes.', | |
| }); | |
| } | |
| } catch (error) { | |
| console.error(`Error reading agents directory: ${error.message}`); | |
| } | |
| return { agents, parseErrors }; | |
| } | |
| async function loadAgentsFromDir(dir, scope, depDirs) { | |
| const agents = []; | |
| const parseErrors = []; | |
| let files; | |
| try { | |
| files = await fs.readdir(dir); | |
| } catch { | |
| return { agents, parseErrors }; // directory absent — nothing to validate | |
| } | |
| for (const file of files) { | |
| if (!file.endsWith('.md') || file.startsWith('_')) continue; | |
| const filePath = path.join(dir, file); | |
| try { | |
| const content = await fs.readFile(filePath, 'utf-8'); | |
| const parsed = extractYamlFromMarkdown(content); | |
| if (parsed?.agent) { | |
| agents.push({ | |
| file, | |
| path: filePath, | |
| scope, | |
| depDirs, | |
| id: parsed.agent.id || file.replace('.md', ''), | |
| name: parsed.agent.name, | |
| commands: parsed.commands || [], | |
| dependencies: parsed.dependencies || {}, | |
| parsed, | |
| }); | |
| } | |
| } catch (error) { | |
| parseErrors.push({ | |
| type: 'INVALID_YAML', | |
| scope, | |
| agent: file.replace('.md', ''), | |
| file, | |
| path: filePath, | |
| message: `Failed to load ${scope}/${file}: ${String(error.message).split('\n')[0]}`, | |
| suggestion: | |
| 'Fix the embedded YAML block. A common cause is a sequence entry like `- "term" (note)`, where the text after the closing quote is a stray token — wrap the whole entry in single quotes.', | |
| }); | |
| } | |
| } | |
| return { agents, parseErrors }; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 136-136: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFile(filePath, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.aiox-core/infrastructure/scripts/validate-agents.js around lines 121 - 168,
The loadAgentsFromDir function should distinguish file read failures from YAML
parsing failures. Handle fs.readFile errors separately with an appropriate
read-error type and message, then keep extractYamlFromMarkdown errors classified
as INVALID_YAML with the existing YAML suggestion.
| function commandName(cmd) { | ||
| if (typeof cmd === 'string') { | ||
| // String format: 'command: description' | ||
| return cmd.split(':')[0].trim(); | ||
| } | ||
| if (cmd && cmd.name) { | ||
| // Explicit format: { name: 'command', ... } | ||
| return cmd.name; | ||
| } | ||
| if (typeof cmd === 'object' && cmd !== null) { | ||
| // Shorthand format: { command: 'description' } - take first key | ||
| return Object.keys(cmd)[0]?.split(' ')[0]; // Handle 'command {args}' format | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target=".aiox-core/infrastructure/scripts/validate-agents.js"
if [ -f "$target" ]; then
echo "FILE_EXISTS: yes"
wc -l "$target"
printf '\n--- lines 200-270 ---\n'
sed -n '200,270p' "$target" | nl -ba -v200
printf '\n--- commandName usages/callers nearby ---\n'
rg -n "commandName\\(|DUPLICATE_COMMAND|validate.*agent|agents" "$target" -C 3
else
echo "FILE_EXISTS: no"
echo "Matching files:"
fd -a "validate-agents.js" . | sed 's#^\./##'
fiRepository: SynkraAI/aiox-core
Length of output: 293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target=".aiox-core/infrastructure/scripts/validate-agents.js"
echo "--- lines 200-270 ---"
sed -n '200,270p' "$target" | awk '{printf "%6d %s\n", NR+199, $0}'
echo
echo "--- relevant searches ---"
for term in "commandName\\(" "DUPLICATE_COMMAND" "validate.*agent"; do
echo "=== $term ==="
grep -En "$term" "$target" -C 2 || true
done
echo
echo "--- minimal JS lexical probe in a temp .js file without executing repo code ---"
tmp="$(mktemp)"
sed -n '1,16p' "$target"
# Since Node/JS runtime may not be available and even if it is, running package code is discouraged,
# inspect the helper implementation via pure shell text for the current change.Repository: SynkraAI/aiox-core
Length of output: 5700
🌐 Web query:
Node.js Object.prototype.hasOwnProperty.call documentation
💡 Result:
In Node.js and other JavaScript environments, Object.prototype.hasOwnProperty.call(obj, prop) is a defensive, reliable way to check if an object has a specific property as its own (non-inherited) property [1][2][3]. While you can often use obj.hasOwnProperty(prop), using the generic method call is recommended to avoid several potential pitfalls [4][5]: 1. Shadowed Properties: If an object has a custom property named hasOwnProperty, calling obj.hasOwnProperty will execute that property instead of the built-in method, potentially causing errors or security vulnerabilities [5][3]. This is particularly risky when dealing with untrusted input (e.g., JSON objects from a web request) [5]. 2. Objects Lacking Prototype: Objects created with Object.create(null) do not inherit from Object.prototype, meaning they do not possess the hasOwnProperty method at all [1][2][5]. Calling obj.hasOwnProperty on these objects will throw a TypeError [2][6][3]. 3. Method Overriding: If the hasOwnProperty method has been redefined or deleted on the object's prototype chain, the standard method call may fail or behave unpredictably [4][3]. Using Object.prototype.hasOwnProperty.call(obj, prop) bypasses these issues by directly invoking the native method from Object.prototype and explicitly setting the 'this' context to the target object [1][3]. Modern Recommendation: In modern JavaScript environments (including recent versions of Node.js), it is recommended to use Object.hasOwn(obj, prop) instead [1][7][6]. This method is cleaner, more readable, and designed specifically to be a safer, built-in replacement for the older, verbose pattern [1][6]. Example: const obj = { key: 'value' }; // Recommended modern approach Object.hasOwn(obj, 'key'); // true // Robust older pattern Object.prototype.hasOwnProperty.call(obj, 'key'); // true // Risky approach (avoid if the object source is untrusted) obj.hasOwnProperty('key'); // true (but unsafe)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwnProperty
- 2: https://transcoding.org/javascript/hasownproperty/
- 3: https://stackoverflow.com/questions/41471310/why-is-hasownproperty-being-invoked-generically-here
- 4: https://stackoverflow.com/questions/37046946/object-prototype-hasownproperty-call-vs-object-prototype-hasownproperty
- 5: https://eslint.org/docs/latest/rules/no-prototype-builtins
- 6: https://stackoverflow.com/questions/12017693/why-use-object-prototype-hasownproperty-callmyobj-prop-instead-of-myobj-hasow
- 7: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwnProperty?redirectlocale=en-US&redirectslug=JavaScript%2FReference%2FGlobal_Objects%2FObject%2FhasOwnProperty
Do not fall through malformed explicit-format commands to the shorthand branch.
A command like { name: '', description: '...' } skips the cmd && cmd.name check and is treated as shorthand, returning the literal key 'name'; different malformed commands can therefore collide under the same bogus command. Guard the explicit format with Object.hasOwn(cmd, 'name') and return cmd.name || undefined so it fails with a missing name instead of creating a duplicate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.aiox-core/infrastructure/scripts/validate-agents.js around lines 235 - 249,
Update commandName to detect explicit-format objects using Object.hasOwn(cmd,
'name'), including empty names, and return cmd.name || undefined. Prevent
malformed explicit commands from reaching the shorthand Object.keys branch and
producing a bogus command name.
| if (depType === 'reference_files') { | ||
| for (const refFile of depList) { | ||
| if (typeof refFile !== 'string') continue; | ||
| const refPath = path.join(REPO_ROOT, refFile); | ||
| if (!(await fileExists(refPath))) { | ||
| warnings.push({ | ||
| type: 'MISSING_REFERENCE_FILE', | ||
| scope: agent.scope, | ||
| agent: agent.id, | ||
| depType, | ||
| depFile: refFile, | ||
| expectedPath: refPath, | ||
| message: `Missing reference file: @${agent.id} → ${refFile}`, | ||
| suggestion: `Create ${refFile} or remove it from the agent's reference_files.`, | ||
| }); | ||
| } | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
reference_files entries can escape REPO_ROOT via ../ traversal.
Unlike file-based deps resolved against a fixed <type> directory, refFile is joined directly onto REPO_ROOT with no containment check. A crafted reference_files entry (e.g. ../../etc/passwd) would have its existence probed outside the repo and the resolved path echoed back in the warning's expectedPath. Given squads can be sourced externally (squad-downloader), it's worth constraining the resolved path to stay under REPO_ROOT.
🛡️ Suggested fix
if (depType === 'reference_files') {
for (const refFile of depList) {
if (typeof refFile !== 'string') continue;
const refPath = path.join(REPO_ROOT, refFile);
+ if (!refPath.startsWith(REPO_ROOT + path.sep)) {
+ continue; // reject paths that escape the repository root
+ }
if (!(await fileExists(refPath))) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (depType === 'reference_files') { | |
| for (const refFile of depList) { | |
| if (typeof refFile !== 'string') continue; | |
| const refPath = path.join(REPO_ROOT, refFile); | |
| if (!(await fileExists(refPath))) { | |
| warnings.push({ | |
| type: 'MISSING_REFERENCE_FILE', | |
| scope: agent.scope, | |
| agent: agent.id, | |
| depType, | |
| depFile: refFile, | |
| expectedPath: refPath, | |
| message: `Missing reference file: @${agent.id} → ${refFile}`, | |
| suggestion: `Create ${refFile} or remove it from the agent's reference_files.`, | |
| }); | |
| } | |
| } | |
| continue; | |
| } | |
| if (depType === 'reference_files') { | |
| for (const refFile of depList) { | |
| if (typeof refFile !== 'string') continue; | |
| const refPath = path.join(REPO_ROOT, refFile); | |
| if (!refPath.startsWith(REPO_ROOT + path.sep)) { | |
| continue; // reject paths that escape the repository root | |
| } | |
| if (!(await fileExists(refPath))) { | |
| warnings.push({ | |
| type: 'MISSING_REFERENCE_FILE', | |
| scope: agent.scope, | |
| agent: agent.id, | |
| depType, | |
| depFile: refFile, | |
| expectedPath: refPath, | |
| message: `Missing reference file: @${agent.id} → ${refFile}`, | |
| suggestion: `Create ${refFile} or remove it from the agent's reference_files.`, | |
| }); | |
| } | |
| } | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.aiox-core/infrastructure/scripts/validate-agents.js around lines 325 - 343,
Constrain `reference_files` resolution in the `depType === 'reference_files'`
block so entries containing traversal cannot escape `REPO_ROOT`. Resolve the
path, validate that it remains within `REPO_ROOT`, and only call `fileExists` or
expose `expectedPath` for contained paths; handle escaping entries as missing or
invalid references using the existing warning flow.
1 participant
📋 Description
Follow-up to #815, which surfaced the gap:
npm run validate:agentsvalidated only the 12 core agents. Squads had no gate at all — that is why four typographic variants of the same guard block and a YAML block no parser accepts coexisted insquads/claude-code-mastery/unnoticed.While extending it, a worse problem showed up in the loader itself:
The
try/catchwraps the entire loop. One unparseable agent aborted the read of every remaining agent, the failure surfaced as aconsole.error, and the run still ended with✅ All validations passed!and exit 0. The gate existed on paper only.📦 Type of Change
🎯 Scope
aiox-core/)📝 Changes Made
Squad coverage. Discovers
squads/<squad>/agents/alongside the core directory, with a per-scope summary:New "YAML Parse" check. An unparseable block is now an error (exit 1) with the reason and a fix hint, and the
try/catchis per file — one broken agent no longer hides the others.Command uniqueness is per scope. Core competes with core, each squad with itself. The same command name in two different squads is not a collision — without this, enabling squads would have produced mass false positives (the squad alone has 97 commands).
Squad dependencies resolve inside the squad (
squads/<squad>/<type>/), andreference_files— which hold repo-relative paths — are checked against the repo root instead of being reported asUNKNOWN_DEP_TYPE.--core-onlyrestricts the run to the previous behaviour.Also removed a dead function the refactor left behind referencing undefined variables.
loadAllAgents()keeps its old signature (returns just the list) so existing consumers don't break;loadAgents()is the new one, returning{ agents, parseErrors, scopes }.🧪 Testing
checklists/+data/dependencies in core, none introduced here.--core-only→ 12 agents, matching the old output.Removing the file returns exit 0. Files starting with
_remain skipped by design.npm run lintpasses.🔗 Related
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation